Skip to content

feat: support realtime toggle update via socket.io #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

feat: support realtime toggle update via socket.io #29

wants to merge 5 commits into from

Conversation

@hezean
Copy link
Contributor

@hezean hezean commented Dec 4, 2022

resolves #27

@hezean hezean marked this pull request as draft December 4, 2022 17:51
@codecov-commenter
Copy link

codecov-commenter commented Dec 4, 2022 •
edited
Loading

Codecov Report

Merging #29 (284fde5) into main (54af79b) will decrease coverage by 0.97%.
The diff coverage is 50.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #29      +/-   ##
============================================
- Coverage     76.74%   75.77%   -0.98%     
- Complexity      268      271       +3     
============================================
  Files            31       31              
  Lines           972     1003      +31     
  Branches         68       70       +2     
============================================
+ Hits            746      760      +14     
- Misses          184      200      +16     
- Partials         42       43       +1
Impacted Files Coverage Δ
...java/com/featureprobe/sdk/server/FeatureProbe.java 64.46% <ø> (ø)
...ain/java/com/featureprobe/sdk/server/FPConfig.java 56.60% <20.00%> (-3.82%) ⬇️
...in/java/com/featureprobe/sdk/server/FPContext.java 66.00% <50.00%> (-1.40%) ⬇️
...m/featureprobe/sdk/server/PollingSynchronizer.java 59.21% <55.17%> (-3.76%) ⬇️

@hezean hezean mentioned this pull request Dec 4, 2022
Closed
@hezean hezean marked this pull request as ready for review December 27, 2022 06:09
@SSebo
Copy link
Member

SSebo commented Dec 28, 2022

Good Job, How about add a Test case to connecting featureprobe.io to make sure socketio connect success?

@hezean
Copy link
Contributor Author

hezean commented Dec 28, 2022 via email

@SSebo
Copy link
Member

SSebo commented Dec 28, 2022 •
edited
Loading

it was manually tested, imho it’s not a good idea to let ut depends on external resourcesBest regards,Zean HeSent from mobile device.On Dec 28, 2022, at 10:49, SSebo @.> wrote: Good Job, How about add a Test case to connecting featureprobe.io to make sure socketio connect success? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.>

You are right, but it is better than no test. Rust SDK test this via an embed Server.
I have two ideas:

  1. Treat it as an integration test, only run in github actions.
  2. Embed js socketio server for test (better but more complex).

@gangb-tech
Copy link
Member

gangb-tech commented Jan 4, 2023

Good, The stream feature should be independent. You can try to abstract a StreamSynchronizer to implement it.

@gangb-tech
Copy link
Member

gangb-tech commented Mar 6, 2023

This request has been merged into #36

@gangb-tech gangb-tech closed this Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: java sdk integrate socketio client to receive realtime toggle update event

4 participants

@hezean @codecov-commenter @SSebo @gangb-tech